Upstream: Add convenience wrappers for common uio usage #10412

lundman · 2020-06-07T00:33:12Z

Use ZFS specific UIO macros for shared-code.
The first controversial PR!

Motivation and Context

Code that poke around inside uio, like uio->l_offset can not work on OsX due to opaqueness of the struct. The API call uio_offset() should be used. I went with this for readability, if people prefer #ifdef APPLE everywhere, that is also doable.

Description

The macOS uio struct is opaque and the API must be used, this
makes the smallest changes to the code for all platforms.

Most affected area is module/icp, which had a lot of code repetition.
It was tricky to make the for loop that skips to first non-empty
iov index look good, so it was moved into a macro. Technically, this
"skip code" is not required in ZFS, as all callers to crypto will not
create empty iov.

For those curious what the macOS versions of the macros look like, they are here:

https://github.com/openzfsonosx/openzfs/blob/sorted2/include/os/macos/zfs/sys/zfs_context_os.h#L69

How Has This Been Tested?

Unable to test, relying on CI.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

lundman · 2020-06-07T06:28:04Z

Is FreeBSD not including zfs_context_os.h when compiling dmu.c?

codecov · 2020-06-07T09:02:38Z

Codecov Report

Merging #10412 into master will decrease coverage by 0.49%.
The diff coverage is 41.81%.

@@            Coverage Diff             @@
##           master   #10412      +/-   ##
==========================================
- Coverage   79.58%   79.09%   -0.50%     
==========================================
  Files         391      393       +2     
  Lines      123872   123859      -13     
==========================================
- Hits        98586    97963     -623     
- Misses      25286    25896     +610

Flag	Coverage Δ
#kernel	`79.97% <38.54%> (-0.06%)`	⬇️
#user	`63.57% <34.73%> (-2.55%)`	⬇️

Impacted Files	Coverage Δ
module/icp/io/sha1_mod.c	`2.43% <0.00%> (+0.06%)`	⬆️
module/icp/io/sha2_mod.c	`29.05% <0.00%> (+0.78%)`	⬆️
module/icp/io/skein_mod.c	`4.91% <0.00%> (+0.15%)`	⬆️
module/zfs/sa.c	`84.30% <ø> (ø)`
module/zfs/zfs_sa.c	`61.11% <0.00%> (ø)`
include/os/linux/spl/sys/uio.h	`100.00% <100.00%> (ø)`
lib/libspl/include/sys/uio.h	`100.00% <100.00%> (ø)`
module/icp/algs/modes/modes.c	`50.94% <100.00%> (-3.61%)`	⬇️
module/icp/core/kcf_prov_lib.c	`63.07% <100.00%> (-0.70%)`	⬇️
module/zfs/dmu.c	`86.19% <100.00%> (-0.24%)`	⬇️
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e08b993...c31e037. Read the comment docs.

include/os/freebsd/zfs/sys/zfs_context_os.h

include/os/freebsd/spl/sys/uio.h

lib/libspl/include/os/freebsd/sys/zfs_context_os.h

lib/libspl/include/os/linux/sys/zfs_context_os.h

lib/libspl/include/sys/uio.h

behlendorf

Just a couple last nits. Let's also run it by @mattmacy to get his feedback.

include/os/freebsd/spl/sys/uio.h

module/icp/core/kcf_prov_lib.c

module/icp/io/skein_mod.c

mattmacy

LGTM

behlendorf · 2020-06-13T17:47:30Z

You definitely got unlucky with the bots. But all the failures are known. If you could rebase this on master we should be able to get a clean run with only the FreeBSD head bot failing due to recent changes they made upstream.

The macOS uio struct is opaque and the API must be used, this makes the smallest changes to the code for all platforms. Signed-off-by: Jorgen Lundman <lundman@lundman.net>

The macOS uio struct is opaque and the API must be used, this makes the smallest changes to the code for all platforms. Reviewed-by: Matt Macy <mmacy@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Jorgen Lundman <lundman@lundman.net> Closes #10412

The macOS uio struct is opaque and the API must be used, this makes the smallest changes to the code for all platforms. Reviewed-by: Matt Macy <mmacy@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Jorgen Lundman <lundman@lundman.net> Closes openzfs#10412

lundman added the Status: Code Review Needed Ready for review and testing label Jun 7, 2020

lundman force-pushed the macOS_pr4 branch from 2de97a1 to 29c504c Compare June 7, 2020 02:28

behlendorf reviewed Jun 9, 2020

View reviewed changes

include/os/freebsd/zfs/sys/zfs_context_os.h Outdated Show resolved Hide resolved

include/os/freebsd/zfs/sys/zfs_context_os.h Outdated Show resolved Hide resolved

lundman force-pushed the macOS_pr4 branch 3 times, most recently from 51d866b to 6d16942 Compare June 9, 2020 06:58

lundman commented Jun 9, 2020

View reviewed changes

include/os/freebsd/spl/sys/uio.h Show resolved Hide resolved

behlendorf reviewed Jun 10, 2020

View reviewed changes

lundman force-pushed the macOS_pr4 branch from 6d16942 to 74382c6 Compare June 11, 2020 01:29

behlendorf approved these changes Jun 11, 2020

View reviewed changes

include/os/freebsd/spl/sys/uio.h Outdated Show resolved Hide resolved

include/os/freebsd/spl/sys/uio.h Outdated Show resolved Hide resolved

module/icp/core/kcf_prov_lib.c Outdated Show resolved Hide resolved

module/icp/io/skein_mod.c Outdated Show resolved Hide resolved

lundman force-pushed the macOS_pr4 branch 3 times, most recently from 522cd82 to 3d47bc6 Compare June 12, 2020 02:58

mattmacy approved these changes Jun 13, 2020

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jun 13, 2020

Upstream: Add convenience wrappers for common uio usage

c31e037

The macOS uio struct is opaque and the API must be used, this makes the smallest changes to the code for all platforms. Signed-off-by: Jorgen Lundman <lundman@lundman.net>

lundman force-pushed the macOS_pr4 branch from 3d47bc6 to c31e037 Compare June 13, 2020 23:19

behlendorf merged commit 883a40f into openzfs:master Jun 14, 2020

lundman deleted the macOS_pr4 branch May 28, 2021 05:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstream: Add convenience wrappers for common uio usage #10412

Upstream: Add convenience wrappers for common uio usage #10412

lundman commented Jun 7, 2020

lundman commented Jun 7, 2020

codecov bot commented Jun 7, 2020 •

edited

Loading

behlendorf left a comment

mattmacy left a comment

behlendorf commented Jun 13, 2020

Upstream: Add convenience wrappers for common uio usage #10412

Upstream: Add convenience wrappers for common uio usage #10412

Conversation

lundman commented Jun 7, 2020

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

lundman commented Jun 7, 2020

codecov bot commented Jun 7, 2020 • edited Loading

Codecov Report

behlendorf left a comment

Choose a reason for hiding this comment

mattmacy left a comment

Choose a reason for hiding this comment

behlendorf commented Jun 13, 2020

codecov bot commented Jun 7, 2020 •

edited

Loading